-
Notifications
You must be signed in to change notification settings - Fork 216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dma: allow choices of CHUNK_SIZE #1889
Conversation
Instead of using a feature, maybe this could be a parameter to
The ESP32-S2 (and the upcoming P4) also allow DMA to/from PSRAM. |
One usability issue there is that the user also has to allocate the descriptors with the same (or less) chunk size to be sure they won't run short of descriptors and panic when they attempt a transfer. (been there done that) I think there needs to be some discussion on how descriptors should be presented. I've not tried but have been thinking something like a |
Yeah the usability is a bit of an issue there. (Though I think features have the same problem)
I'm currently working on something similar in #1856, the difference is the structs I've introduced contain both a descriptor and a buffer together. Eventually I was gonna throw in a |
Sorry for letting this fall off the radar for so long. I think the functionality being added is good, however I'm not sure that creating new features is the correct path forward. I think this boils down to yet another thing that is blocked by #1111, so hopefully we can make some progress there soon. @MabezDev @bjoernQ please let me know if you disagree with this assessment. |
I'd say sooner or later we definitely want users to have more control over the descriptors including the chunk-size. I agree a feature is easy to implement and somewhat easy to use. Downside of a feature is that this CHUNK_SIZE is then used everywhere (not a new downside - it's like that now - users just don't have a choice) Maybe making CHUNK_SIZE a DEFAULT_CHUNK_SIZE and make the macros to allocate buffers and descriptors optionally take a chunk-size would mitigate the annoyance of having the chunk size a parameter |
I like this - I'll rework the PR (or close this one and create a new one) |
Just converting this to a draft until the rework is done, no rush of course :) |
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Description
If you need to DMA to/from PSRAM the descriptor buffer size (CHUNK_SIZE) needs to be a multiple of the cache block size. The default value 4092 is 4 byte aligned and not 32 (a common cache block size) or 64 (an allowable cache size). This PR adds features that change the default CHUNK_SIZE to 4064 (32 byte multiple) or 4032 (64 byte multiple).
I'm not sure that this is the best approach, it is the easiest. An alternative would be to make the
DmaDescriptor
or aDmaDescriptorList
be CHUNK_SIZE aware.Testing
DMA from PSRAM to hub75 panel via LCD peripheral in i8080 mode.